[Serve] Add actor id and worker id to Serve structured logs #43725
Conversation
Signed-off-by: Gene Su <[email protected]>
|
Also tested manually seeing those new fields are now logged |
| for field in ServeJSONFormatter.ADD_IF_EXIST_FIELDS: | ||
| if field in record_attributes: | ||
| record_format[field] = record_attributes[field] |
There was a problem hiding this comment.
I believe the attributes added here can be included in the base record_format that's generated in the constructor
There was a problem hiding this comment.
That would require us to pass more things to the constructor, but I guess it makes more sense there since they won't change between each log messages. Let me update it :)
There was a problem hiding this comment.
These ones are just calling get_runtime_context() anyways
There was a problem hiding this comment.
can't you just call it in there?
There was a problem hiding this comment.
Oh that's right. Let me just call those in init 😅
Signed-off-by: Gene Su <[email protected]>
Signed-off-by: Gene Su <[email protected]>
…or_id() and get_worker_id() in case if they fails Signed-off-by: Gene Su <[email protected]>
| SERVE_LOG_ACTOR_ID: runtime_context.get_actor_id(), | ||
| SERVE_LOG_WORKER_ID: runtime_context.get_worker_id(), | ||
| } | ||
| try: | ||
| runtime_context = ray.get_runtime_context() | ||
| actor_id = runtime_context.get_actor_id() | ||
| if actor_id: | ||
| self.component_log_fmt[SERVE_LOG_ACTOR_ID] = actor_id | ||
| worker_id = runtime_context.get_worker_id() | ||
| if worker_id: | ||
| self.component_log_fmt[SERVE_LOG_WORKER_ID] = worker_id | ||
| except Exception: | ||
| # If get_runtime_context() fails for any reason, do nothing (no adding | ||
| # actor_id and/or worker_id to the fmt) | ||
| pass |
There was a problem hiding this comment.
I think you probably meant to put the try-except block above the dict construction :)
There was a problem hiding this comment.
oh, I meant to remove them in line 68-69 and just have they added to the dictionary in the try-except block 😅
Signed-off-by: Gene Su <[email protected]>
| f'"request_id": "{resp["request_id"]}", ' | ||
| f'"route": "{resp["route"]}", ' | ||
| f'"application": "{resp["app_name"]}", "message":.* user func.*' |
There was a problem hiding this comment.
why are the message fields removed?
There was a problem hiding this comment.
It's just reordered to before the request id. This is due to my refactor for drying up the ADD_IF_EXIST_FIELDS thing. If we do want message to go last I can do move it down. Just feel since it's structured json, the order shouldn't matter
Signed-off-by: Gene Su <[email protected]>
Why are these changes needed?
Add
actor_idandworker_idto Serve structured json logs.Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.